Skip to content

Conversation

cosmic-pixel-painter
Copy link

@cosmic-pixel-painter cosmic-pixel-painter commented Sep 24, 2025

This PR fixes this Typescript issue.

Originally, I created this PR, but closed it as the that approach wasn't ideal.

I started with Omit<GlobalEventHandlers, 'onerror'> as an approach. However, I quickly realized it isn't the correct path and creates a whole slew of new problems due to how event handlers are mapped to different elements during the emit phase. (if you're curious to see what exactly I ran into, I can push up a separate PR).

What this change does:

  • Introduces a new type DocumentOrGlobalOnErrorHandler which extends the OnErrorEventHandlerNonNull type with HTMLElement specific handlers. This type then overrides GlobalEventHandlers onerror property. This allow us to be backwards compatible and fix the bug.

  • This did require a change to both types.ts and emitter.ts so that exposed was accounted for when creating a new typedef. I needed the new DocumentOrGlobalOnErrorHandler to only be exposed on the dom library.

  • the onerror of GlobalEventHandlers cannot be overridden by the overrideType.ts file. I tried to a few times, in both mixin-ins and interfaces. It turns out it has to be overidden in the patches/events.kdl file.

Let me know if I missed anything or another approach would be better. I think this threads the needle nicely as it works only on dom elements and still provides both the global and HTMLElement specific onerror handling types.

Copy link
Contributor

Thanks for the PR!

This section of the codebase is owned by @saschanaz - if they write a comment saying "LGTM" then it will be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HTMLElement inherits onerror and addListener("error" definitions that are unique to Window

2 participants